plugin/scripts: Windows (Git Bash) support#9
Merged
yyiilluu merged 3 commits intoReflexioAI:mainfrom May 3, 2026
Merged
Conversation
📝 WalkthroughWalkthroughAdds Windows-aware process helpers and Python resolver, and refactors service/start/stop and installer scripts to use them for cross-platform detached spawning, PID recording, and process-tree termination. ChangesCross-platform process lifecycle
Sequence Diagram(s)sequenceDiagram
participant ServiceScript as Service script
participant Helper as claude_smart_* helpers
participant OS as Operating System / Process table
ServiceScript->>Helper: claude_smart_spawn_detached(cmd...)
Helper->>OS: spawn detached process (setsid/nohup/Win)
OS-->>Helper: child PID
Helper-->>ServiceScript: returns svc_pid
ServiceScript->>OS: write svc_pid to PID file
ServiceScript->>OS: probe /health
ServiceScript->>Helper: claude_smart_kill_tree(svc_pid)
Helper->>OS: taskkill or kill (TERM -> wait -> KILL)
OS-->>Helper: termination result
Helper-->>ServiceScript: exit status
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Review rate limit: 9/10 reviews remaining, refill in 6 minutes. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugin/scripts/_lib.sh`:
- Around line 42-59: The POSIX branch of claude_smart_resolve_python currently
only checks discoverability with command -v but must also verify the interpreter
actually runs like the Windows branch; update the for-loop for cand in python3
python to, after command -v "$cand", attempt a lightweight runtime check (e.g.,
"$cand" -V or --version redirected to /dev/null) and only return that candidate
if the check succeeds so that claude_smart_spawn_detached() will receive a
working interpreter; keep the same candidate order and error handling (return 1
if none pass).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 18d623bb-c99a-403a-a30b-f6680f6fd22a
📒 Files selected for processing (4)
plugin/scripts/_lib.shplugin/scripts/backend-service.shplugin/scripts/dashboard-service.shplugin/scripts/smart-install.sh
Five Git-Bash-on-Windows bugs prevented the plugin install + service auto-start from working. None of them affect Linux/macOS — every change is gated on `claude_smart_is_windows` (uname matches MINGW*/MSYS*/CYGWIN*). 1. uv install: the astral.sh bash installer downloads a zip and unzips it, but Git Bash's bundled unzip corrupts the Windows uv.exe (bad CRC on the inflated binary). On Windows, run the official PowerShell installer (install.ps1) instead — same destination (~/.local/bin/uv.exe), so the post-install PATH prepend works uniformly. 2. python3 App Execution Alias stub: on Windows, %LocalAppData%\Microsoft\WindowsApps\python3.exe is a Microsoft Store launcher stub. `command -v python3` returns truthy but invoking it prints "Python was not found" and exits non-zero. Add claude_smart_resolve_python which probes interpreters with `-V` to filter the stub out, preferring `python` (the real installed one) over `python3` on Windows. Used by smart-install.sh's settings.json patcher and by the spawn-detached fallback. 3. spawn_detached on Windows: Git Bash has no setsid, no process groups, and os.setsid() is POSIX-only — the existing python3-based fallback would never work even if a real Python were on PATH. Use plain nohup on Windows: ignoring SIGHUP is sufficient for the child to survive the parent console closing, and Windows doesn't have the zombie-on-parent-exit problem POSIX has. 4. kill_tree on Windows: kill -TERM -- "-pgid" (negative pid = process group) is a POSIX construct with no Windows equivalent. Use taskkill /T /F /PID, which walks the child-process tree via the parent-pid/job-object relationships. Subtlety: in Git Bash, $! for a backgrounded job returns the MSYS pid (an internal counter), not the native Windows pid taskkill needs. ps's default output exposes the WINPID column; awk extracts it. Service scripts now delegate kill_group to claude_smart_kill_tree so the logic stays in one place. 5. Service start scripts (backend-service.sh, dashboard-service.sh): the inline setsid/python3/nohup detach blocks would land in the broken python3 branch on Windows. Replace with the unified claude_smart_spawn_detached helper. PID-file recording is split: Windows records the spawned MSYS pid (translated to WINPID by kill_tree at signal time); POSIX still records the pgid where setsid guarantees pid==pgid, and falls back to ps -o pgid otherwise. Tested on Windows 11 / Git Bash (MINGW64): smart-install.sh now completes (uv install + uv sync + dashboard build), and claude_smart_spawn_detached + kill_tree round-trip a `ping -n 100` through start-alive-kill-gone end-to-end. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugin/scripts/dashboard-service.sh (1)
129-139:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid the PGID race on the Python-detach path.
When
setsidis unavailable butclaude_smart_spawn_detacheduses the Pythonos.setsid()branch, theps -o pgid=lookup here can run before the child has entered its new session. That can persist the parent PGID, sostopmay miss the dashboard or signal the wrong process group on macOS/fallback POSIX.💡 Suggested fix
claude_smart_spawn_detached npm run start >>"$LOG_FILE" 2>&1 dash_pid=$! if claude_smart_is_windows; then echo "$dash_pid" > "$PID_FILE" elif command -v setsid >/dev/null 2>&1; then echo "$dash_pid" > "$PID_FILE" + elif _CS_PY=$(claude_smart_resolve_python) && [ -n "$_CS_PY" ]; then + # claude_smart_spawn_detached used python os.setsid(), so pid==pgid. + echo "$dash_pid" > "$PID_FILE" else actual_pgid="" if command -v ps >/dev/null 2>&1; then actual_pgid=$(ps -o pgid= -p "$dash_pid" 2>/dev/null | tr -d ' ') fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/scripts/dashboard-service.sh` around lines 129 - 139, The current PGID lookup can race when the Python detach path (claude_smart_spawn_detached using os.setsid()) is used: after starting the dashboard (dash_pid) the ps -o pgid= query may run before the child joins its new session and return the parent's PGID. Change the fallback branch (when setsid is unavailable and claude_smart_is_windows is false) to poll for the child's PGID: repeatedly run ps -o pgid= -p "$dash_pid" with a short sleep until the returned PGID differs from the original parent PGID (or until a short timeout), then write that PGID (or dash_pid if timeout/no ps) to PID_FILE; this ensures the stop logic targets the correct process group when claude_smart_spawn_detached used Python os.setsid().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@plugin/scripts/dashboard-service.sh`:
- Around line 129-139: The current PGID lookup can race when the Python detach
path (claude_smart_spawn_detached using os.setsid()) is used: after starting the
dashboard (dash_pid) the ps -o pgid= query may run before the child joins its
new session and return the parent's PGID. Change the fallback branch (when
setsid is unavailable and claude_smart_is_windows is false) to poll for the
child's PGID: repeatedly run ps -o pgid= -p "$dash_pid" with a short sleep until
the returned PGID differs from the original parent PGID (or until a short
timeout), then write that PGID (or dash_pid if timeout/no ps) to PID_FILE; this
ensures the stop logic targets the correct process group when
claude_smart_spawn_detached used Python os.setsid().
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1b54716f-2c52-4e69-8b8e-6f0ed5f2e8fa
📒 Files selected for processing (4)
plugin/scripts/_lib.shplugin/scripts/backend-service.shplugin/scripts/dashboard-service.shplugin/scripts/smart-install.sh
🚧 Files skipped from review as they are similar to previous changes (3)
- plugin/scripts/_lib.sh
- plugin/scripts/backend-service.sh
- plugin/scripts/smart-install.sh
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugin/scripts/_lib.sh`:
- Around line 24-33: The claude_smart_is_windows() helper incorrectly treats
CYGWIN as Windows; narrow its case pattern to only MINGW* and MSYS* so CYGWIN
falls through to the POSIX path—update the case in claude_smart_is_windows() to
match only MINGW*|MSYS* and leave CYGWIN* out so callers like
claude_smart_spawn_detached() and claude_smart_kill_tree() will use the POSIX
behavior (setsid/process groups and TERM→KILL semantics) instead of taskkill.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5989a3e1-8971-450e-af46-4629d40643f7
📒 Files selected for processing (3)
plugin/scripts/_lib.shplugin/scripts/backend-service.shplugin/scripts/dashboard-service.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- plugin/scripts/dashboard-service.sh
Comment on lines
+24
to
+33
| # Return 0 (true) if running under a Windows-flavoured bash (Git Bash, | ||
| # MSYS, Cygwin). Used to gate POSIX-only primitives (setsid, process | ||
| # groups) and route around Windows-specific potholes (the python3 App | ||
| # Execution Alias stub at WindowsApps\python3.exe). | ||
| claude_smart_is_windows() { | ||
| case "$(uname -s 2>/dev/null)" in | ||
| MINGW*|MSYS*|CYGWIN*) return 0 ;; | ||
| *) return 1 ;; | ||
| esac | ||
| } |
There was a problem hiding this comment.
Route Cygwin through the POSIX code path, not Windows.
CYGWIN* fully supports POSIX setsid, process groups, and signal-based termination. Routing it through MINGW*|MSYS* branches in claude_smart_spawn_detached() and claude_smart_kill_tree() breaks this: the Windows branch uses taskkill, a Windows-native tool that cannot reliably terminate POSIX Cygwin processes, and it loses the graceful TERM→KILL semantics. Narrow claude_smart_is_windows to MINGW*|MSYS* only.
Suggested change
claude_smart_is_windows() {
case "$(uname -s 2>/dev/null)" in
- MINGW*|MSYS*|CYGWIN*) return 0 ;;
+ MINGW*|MSYS*) return 0 ;;
*) return 1 ;;
esac
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| # Return 0 (true) if running under a Windows-flavoured bash (Git Bash, | |
| # MSYS, Cygwin). Used to gate POSIX-only primitives (setsid, process | |
| # groups) and route around Windows-specific potholes (the python3 App | |
| # Execution Alias stub at WindowsApps\python3.exe). | |
| claude_smart_is_windows() { | |
| case "$(uname -s 2>/dev/null)" in | |
| MINGW*|MSYS*|CYGWIN*) return 0 ;; | |
| *) return 1 ;; | |
| esac | |
| } | |
| # Return 0 (true) if running under a Windows-flavoured bash (Git Bash, | |
| # MSYS, Cygwin). Used to gate POSIX-only primitives (setsid, process | |
| # groups) and route around Windows-specific potholes (the python3 App | |
| # Execution Alias stub at WindowsApps\python3.exe). | |
| claude_smart_is_windows() { | |
| case "$(uname -s 2>/dev/null)" in | |
| MINGW*|MSYS*) return 0 ;; | |
| *) return 1 ;; | |
| esac | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugin/scripts/_lib.sh` around lines 24 - 33, The claude_smart_is_windows()
helper incorrectly treats CYGWIN as Windows; narrow its case pattern to only
MINGW* and MSYS* so CYGWIN falls through to the POSIX path—update the case in
claude_smart_is_windows() to match only MINGW*|MSYS* and leave CYGWIN* out so
callers like claude_smart_spawn_detached() and claude_smart_kill_tree() will use
the POSIX behavior (setsid/process groups and TERM→KILL semantics) instead of
taskkill.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes five Windows-specific bugs in the plugin shell scripts so
claude-smartinstalls and auto-starts cleanly under Git Bash (MINGW64) on Windows 10/11. Every change is gated onclaude_smart_is_windows(uname matchesMINGW*/MSYS*/CYGWIN*), so Linux and macOS behaviour is byte-for-byte unchanged.Hit while installing the plugin on Windows 11 with
claude-smart0.1.17. The dashboard's/claude-smart:dashboardslash command failed withbackend: not runninganddashboard: .next missing, because the install never completed and the service-start scripts couldn't detach.What was broken
smart-install.shuv installuvinstall reports success thenuv syncfails becauseuv.exeis corruptinstall.sh) downloads a zip and unzips it; Git Bash's bundledunzipcorrupts the Windowsuv.exe(bad CRC on inflate).smart-install.shcs-cite heredocpython3is the Microsoft Store App Execution Alias stub atWindowsApps\python3.exe.command -v python3returns truthy but invoking it prints "Python was not found" and exits non-zero._lib.shclaude_smart_spawn_detachedpython3 -c '...os.setsid()...'— same App Execution stub fires; even with a real Python,os.setsid()is POSIX-only and wouldAttributeError.backend-service.sh/dashboard-service.shkill_groupstopdoesn't kill the servicekill -TERM -- "-pgid"(negative pid = process group) is POSIX-only. Windows has no process groups, so the signal goes nowhere.What changes
_lib.sh— three new helpers, one rewritten:claude_smart_is_windows—uname -stest forMINGW*|MSYS*|CYGWIN*.claude_smart_resolve_python— probes interpreters with-Vto filter the App Execution Alias stub. Preferspythonoverpython3on Windows; preservespython3-first ordering on POSIX.claude_smart_kill_tree— POSIX: signal the process group (TERM, grace, KILL). Windows:taskkill //T //F //PID. Walks Git Bash's MSYS-pid to Windows-pid translation viaps(column 4 in default output isWINPID);$!for a backgrounded job returns the MSYS pid, buttaskkillneeds the native Windows pid.claude_smart_spawn_detached— Windows path: plainnohup(no setsid, no process groups; nohup ignores SIGHUP, which is sufficient for the child to survive the parent console closing). POSIX path: setsid → resolved-pythonos.setsid()→ nohup, with the python branch now gated on-Vso the App Execution stub never fires.smart-install.sh— Windows branch in the uv-install block uses the official PowerShell installer (irm https://astral.sh/uv/install.ps1 | iex); same destination (~/.local/bin/uv.exe), so the post-installPATHprepend works uniformly. The cs-cite settings.json patcher usesclaude_smart_resolve_pythoninstead of barepython3. Also adds~/.local/bin/uv.exeto the post-install fallback search.backend-service.sh/dashboard-service.sh— drop the inline setsid/python3/nohup chains; callclaude_smart_spawn_detachedand record$!. PID-file logic split per-OS:claude_smart_kill_treetranslates to WINPID at signal time.ps -o pgidas before.kill_groupbecomes a one-line wrapper aroundclaude_smart_kill_treeso the cross-OS logic lives in one place.What's intentionally out of scope
reap_port_listeners(backend-service.sh) and the dashboardstopfallback both uselsof, which isn't on Git Bash by default. Existingcommand -v lsofguards already make these no-ops on Windows. Replacing them withnetstat -ano | findstr LISTENINGparsing would work but expands surface area. The recorded-pid path withtaskkill /T /Falready covers the normal stop case (killsnpmplus itsnodechildren).uv sync --python <ver>: while building this PR I hit a separate failure whereuvdefaulted to Python 3.14 andhdbscan(transitive viareflexio-ai) has no 3.14 wheel for Windows, so the source build fired and failed on a 3.14 incompatibility. That's a transitive-dep issue, not a Windows-bash issue, and would also bite Linux/macOS users who install Python 3.14. Worth fixing upstream (probably by pinning inpyproject.tomlor via a--pythonflag), but I left it out so this PR stays focused on the bash-vs-Windows angle..envfile encoding: reflexio's.envscaffolding wrote a CP1252-encoded em-dash (byte0x97) into a comment, whichpython-dotenvthen rejected on the backend at startup. Repository for that fix isReflexioAI/reflexio, not this one.Test plan
bash -nclean on all four edited scripts.claude_smart_is_windowsreturns 0 on Git Bash MINGW64 (Windows 11), 1 on Linux/macOS shells.claude_smart_resolve_pythonreturns the realpythoninterpreter on Windows when both real-Python and the WindowsApps stub are on PATH; returnspython3on POSIX.claude_smart_spawn_detached+claude_smart_kill_treeround-trip: spawnping -n 100 127.0.0.1, verifykill -0reports alive, verifytasklistshowsPING.EXEwith the matching WINPID, kill via helper, confirm process gone fromtasklistandkill -0reports dead.smart-install.shend-to-end on Windows 11 / Git Bash:uvinstalls via PowerShell (no unzip corruption),uv synccompletes (with Python 3.12 — see the "out of scope" note above), dashboardnpm install+npm run buildproduce a working.next/.reflexio services start --only backend --no-reload) and dashboard (npm run start) reach/healthafter manual launch via the helpers;/claude-smart:dashboardopenshttp://localhost:3001and renders.claude_smart_is_windowsso behaviour should be unchanged, but a maintainer with those environments should re-run the existing service start/stop tests to confirm.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes